This repository was archived by the owner on Mar 4, 2020. It is now read-only.
[READY FOR REVIEW] feat(Button): adding disabled prop to Button#14
Merged
[READY FOR REVIEW] feat(Button): adding disabled prop to Button#14
Conversation
bmdalex
requested review from
dvdzkwsk,
kuzhelov and
levithomason
as code owners
Codecov Report
@@ Coverage Diff @@
## master #14 +/- ##
==========================================
- Coverage 82.77% 82.73% -0.04%
==========================================
Files 59 59
Lines 830 840 +10
Branches 186 180 -6
==========================================
+ Hits 687 695 +8
- Misses 139 141 +2
Partials 4 4
Continue to review full report at Codecov.
|
11 tasks
bmdalex
changed the title
Member
The icon should not need to be styled individually here, I don't think... |
11 tasks
bmdalex
force-pushed
the
feat/button-disabled-prop
branch
from
52db469 to
e6e2629
Compare
bmdalex
force-pushed
the
feat/button-disabled-prop
branch
4 times, most recently
from
27c19ef to
c097d5a
Compare
notandrew
reviewed
Jul 31, 2018
Contributor
There was a problem hiding this comment.
If we want the icon color to be the same as the text color, we can use 'currentColor' instead of the hard-coded value.
Collaborator
Author
There was a problem hiding this comment.
@notandrew
currentColor would be a hex number right now and we're not supporting that for Icon component yet. I think @kuzhelov has an ongoing PR for that; we'll address this afterwards
fyi @mnajdova
bmdalex
force-pushed
the
feat/button-disabled-prop
branch
from
c097d5a to
49b0924
Compare
bmdalex
changed the title
Collaborator
Author
bmdalex
force-pushed
the
feat/button-disabled-prop
branch
from
49b0924 to
975e3ce
Compare
bmdalex
force-pushed
the
feat/button-disabled-prop
branch
from
975e3ce to
86f23b2
Compare
mnajdova
approved these changes
Jul 31, 2018
Contributor
mnajdova
left a comment
mnajdova
left a comment
There was a problem hiding this comment.
Good to be merged here.
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Button(
disabledprop)This PR will introduce possibility to specify
disabledbuttonsTODO
API Proposal
disabled
disabledproperty will disable buttons by manipulatingopacityCSS style, borders, colors, hover styles and preventing clickingKnow issue: Icons are not disabled because
disabledproperty was not added to icons at the time of creating this PR. I created a PR for that #113Icon buttons will become disabled out of the box after merging that one
will render